Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding implementation and tests for admin write commands. #252

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

bdchatham
Copy link
Contributor

Adding implementation and tests for admin write commands.

What Changed?

  • Implementation of RemovePendingAdmin
  • Implementation of AddPendingAdmin
  • Implementation of AcceptAdmin
  • implementation of RemoveAdmin
  • Tests for all three commands

&flags.NetworkFlag,
&flags.EnvironmentFlag,
&flags.ETHRpcUrlFlag,
&PermissionControllerAddressFlag,
}
sort.Sort(cli.FlagsByName(cmdFlags))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did not realize before but you should append signer flags first and then sort - so that all flags are sorted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna pick this up in the flag review.

func acceptFlags() []cli.Flag {
cmdFlags := []cli.Flag{
&flags.VerboseFlag,
&AccountAddressFlag,
&flags.OutputTypeFlag,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you going to add broadcast flag and corresponding feature later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, going to do a sweep over all flags in a separate PR.

return nil, eigenSdkUtils.WrapError("failed to create new eth client", err)
}
elWriter, err := common.GetELWriter(
config.AccountAddress,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just return common.GetELWriter (..) instead of creating vars which you return in next line.

@bdchatham bdchatham merged commit b2a7aa7 into madhur/slashing-allocations Dec 11, 2024
3 of 6 checks passed
@bdchatham bdchatham deleted the chatham/admin-write-commands branch December 11, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants